-
Notifications
You must be signed in to change notification settings - Fork 74
[#709] preserve options in chaining obj.metadata(opt1=val1)(opt2=val2) #716
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Conversation
korydraughn
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good overall.
ef597fa to
087ded2
Compare
|
Evidently it is still possible to fail here: The modify time turned out on this occasion to be one second later than the access time on a just-created replica open for write: @korydraughn any ideas why this might occasionally happen? |
|
Squashed , ready for review. |
alanking
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I am especially interested in the changes related to #768 given the milestone for that issue.
Hmm, I don't think the python test code is equivalent to the following C++ test code? The C++ test code I linked to expects the replica to be in the intermediate state. I think the problem with the python test is that the following call closes the data object, resulting in the close operation updating the mtime. python-irodsclient/irods/test/data_obj_test.py Line 3311 in 94b8594
What you need to do is open (and create) a new replica and do your checks while the replica is open. Closing the replica is what causes the atime and mtime to desync. |
Ah, you're right. I see. Will try it a different way. |
|
What's the status of this PR? |
Looks like we are good. Need a squash, and perhaps a codacy review. |
korydraughn
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good overall.
Just a few comments on the README.
b673bdc to
dfb0cd9
Compare
|
To this subject d_bin_meta = d.metadata(iRODSMeta_type = iRODSBinOrStringMeta)
d_bin_meta.set ( 'key1', b'arbitrary-octets-\xe1\x80')And later: At worst, it's the client lib doing too much. At best, it smooths a path that Python2 folks hypothetically might have relied on in the past (since bytestrings and strings were the same back then). |
korydraughn
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This looks ready to me.
Squash to taste once the review comment is resolved.
|
I see a revert in the git history? What's the status of this PR? |
The revert was merely to remove an unrelated change, a commit that shouldn't have made it in.... |
…val2) In other words, setting opt2 = val2 does not reset opt1 back to its default value.
|
Squashed just now. |
|
Okay. Please resolve the ruff-check/format reports. The "Files Changed" page (diff) should highlight the lines reported by the ruff workflows. You can use that view with the github actions report. |
In other words, setting opt2 = val2 does not reset opt1 back to its default value.